feat: SchemaCommitment component
#2253
Conversation
igamigo
force-pushed
the
igamigo-schema-commit
branch
from
abbe163 to
9aa118c
Compare
…iden/miden-base into igamigo-schema-component
There was a problem hiding this comment.
Pull request overview
This PR introduces a new AccountSchemaCommitment component that enables accounts to expose their storage schema commitments on-chain. The component stores a cryptographic commitment to the account's storage schemas in a designated storage slot and provides a procedure to retrieve this commitment.
Changes:
- Added new
AccountSchemaCommitmentcomponent to compute and store deterministic storage schema commitments - Implemented schema commitment computation with lexicographic ordering for order-independence
- Added MASM procedure
get_schema_commitmentto retrieve the schema commitment from account storage
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/miden-standards/src/account/mod.rs | Exports the new metadata module |
| crates/miden-standards/src/account/metadata/mod.rs | Implements AccountSchemaCommitment component with schema commitment computation |
| crates/miden-standards/src/account/components/mod.rs | Adds storage_schema_library() function to access the metadata library |
| crates/miden-standards/asm/account_components/metadata/storage_schema.masm | Implements get_schema_commitment MASM procedure |
| CHANGELOG.md | Documents the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Outdated
Show resolved
Hide resolved
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Outdated
Show resolved
Hide resolved
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Show resolved
Hide resolved
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Outdated
Show resolved
Hide resolved
mmagician
left a comment
mmagician
left a comment
There was a problem hiding this comment.
Whether for now or for another PR, TBD, but it would be useful to also have an option (perhaps default to true) to set AccountBuilder::with_schema(boolean) to indicate whether this component should be built automatically at the time of account creation.
As discussed here, it might be a little more tricky than originally envisioned because at the time when we'd call AccountBuilder::with_schema(boolean), I think we've lost the metadata already.
But, we should have some way (with account builder or otherwise) to conveniently create this component, otherwise I don't see much value in doing all the pre-requisite steps in #2104.
As I mentioned in the second paragraph of this comment, you are already working with components at that point, which do not have metadata associated directly (you might have gotten the component by building it yourself, without |
|
Created #2269 with a possible approach cc @mmagician, left a couple of caveats in the description |
crates/miden-standards/asm/account_components/metadata/schema_commitment.masm
Show resolved
Hide resolved
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comments inline - the main one is about the methodology for combining schemas.
crates/miden-standards/asm/account_components/metadata/schema_commitment.masm
Show resolved
Hide resolved
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comments inline. Once these are addressed, we should be good to merge.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comments for follow-ups - but I'll merge as is.
| /// # Errors | ||
| /// | ||
| /// Returns an error if the schemas contain conflicting definitions for the same slot name. | ||
| pub fn new(schemas: &[StorageSchema]) -> Result<Self, AccountComponentTemplateError> { |
There was a problem hiding this comment.
I wonder if some other error type would make more sense to return from here instead of AccountComponentTemplateError - but also, don't have a good suggestion. Let's add it to the list of follow-ups.
Adds
AccountSchemaCommitmentcomponent, which exposes a single storage slot (namedmiden::standards::metadata::storage_schema) and contains a commitment to the schema of the account, given by the hash of ordered the list ofStorageSchemacommitments - one per component - introduced in #2244.A follow-up (#2269) was created for adding metadata to account component in a way that makes it easy to leverage on
AccountBuilder.